Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dotnet] Migration from Newtonsoft.Json to System.Text.Json package #14292

Merged
merged 51 commits into from
Jul 25, 2024

Conversation

nvborisenko
Copy link
Member

@nvborisenko nvborisenko commented Jul 22, 2024

User description

This is long awaited technical debt.

Description

  • Rewrote underlying serialization/deserialization logic to new package
  • Introduced net8.0 target framework - free of dependencies
  • netstandard2.0 target framework is still dependent on System.Text.Json
  • Use .net8 SDK internally to build projects

Motivation and Context

Fixes #13097

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

All existing tests passed:
image


PR Type

Enhancement, Configuration changes, Dependencies, Tests


Description

  • Replaced Newtonsoft.Json with System.Text.Json for serialization/deserialization across multiple classes.
  • Updated method signatures and JSON parsing logic to use System.Text.Json.
  • Introduced net8.0 target framework and updated dependencies accordingly.
  • Updated various test projects to target net8.0.
  • Updated custom JSON converters and attributes to use System.Text.Json.

Changes walkthrough 📝

Relevant files
Enhancement
27 files
DevToolsSession.cs
Migrate DevToolsSession to System.Text.Json                           

dotnet/src/webdriver/DevTools/DevToolsSession.cs

  • Replaced Newtonsoft.Json with System.Text.Json for
    serialization/deserialization.
  • Updated method signatures to use JsonNode instead of JToken.
  • Adjusted JSON parsing logic to use JsonSerializer.
  • +25/-25 
    ResponseValueJsonConverter.cs
    Update ResponseValueJsonConverter to use System.Text.Json

    dotnet/src/webdriver/Internal/ResponseValueJsonConverter.cs

  • Replaced Newtonsoft.Json with System.Text.Json for custom JSON
    converter.
  • Updated methods to use Utf8JsonReader and Utf8JsonWriter.
  • +56/-54 
    Proxy.cs
    Update Proxy class to use System.Text.Json attributes       

    dotnet/src/webdriver/Proxy.cs

  • Replaced JsonProperty with JsonPropertyName and JsonIgnore attributes.
  • Removed JsonObject attribute.
  • +20/-12 
    Cookie.cs
    Update Cookie class to use System.Text.Json attributes     

    dotnet/src/webdriver/Cookie.cs

  • Replaced JsonProperty with JsonPropertyName and JsonIgnore attributes.
  • Removed JsonObject attribute.
  • +15/-10 
    SeleniumManager.cs
    Migrate SeleniumManager to System.Text.Json                           

    dotnet/src/webdriver/SeleniumManager.cs

  • Replaced Newtonsoft.Json with System.Text.Json for JSON operations.
  • Updated method signatures and JSON parsing logic.
  • +27/-16 
    Command.cs
    Update Command class to use System.Text.Json attributes   

    dotnet/src/webdriver/Command.cs

  • Removed constructor using Newtonsoft.Json.
  • Replaced JsonProperty with JsonPropertyName attributes.
  • +6/-27   
    DevToolsCommandData.cs
    Migrate DevToolsCommandData to System.Text.Json                   

    dotnet/src/webdriver/DevTools/DevToolsCommandData.cs

  • Replaced JToken with JsonNode for command parameters and results.
  • Updated JSON attributes to use System.Text.Json.
  • +11/-10 
    DevToolsVersionInfo.cs
    Update DevToolsVersionInfo to use System.Text.Json attributes

    dotnet/src/webdriver/DevTools/DevToolsVersionInfo.cs

  • Replaced JsonProperty with JsonPropertyName attributes.
  • Added JsonInclude attributes.
  • +13/-7   
    Response.cs
    Migrate Response class to System.Text.Json                             

    dotnet/src/webdriver/Response.cs

  • Replaced Newtonsoft.Json with System.Text.Json for JSON operations.
  • Updated FromJson and ToJson methods.
  • +22/-4   
    ChromiumNetworkConditions.cs
    Update ChromiumNetworkConditions to use System.Text.Json attributes

    dotnet/src/webdriver/Chromium/ChromiumNetworkConditions.cs

  • Replaced JsonProperty with JsonPropertyName and JsonIgnore attributes.
  • Removed JsonObject attribute.
  • +7/-6     
    JsonEnumMemberConverter.cs
    Add JsonEnumMemberConverter for System.Text.Json                 

    dotnet/src/webdriver/DevTools/Json/JsonEnumMemberConverter.cs

  • Added new JSON converter for enum members using System.Text.Json.
  • +58/-0   
    IDevToolsSession.cs
    Update IDevToolsSession to use System.Text.Json                   

    dotnet/src/webdriver/DevTools/IDevToolsSession.cs

    • Replaced JToken with JsonNode in method signatures.
    +4/-4     
    DomMutationData.cs
    Update DomMutationData to use System.Text.Json attributes

    dotnet/src/webdriver/DomMutationData.cs

    • Replaced JsonProperty with JsonPropertyName attributes.
    +5/-5     
    StackTraceElement.cs
    Update StackTraceElement to use System.Text.Json attributes

    dotnet/src/webdriver/StackTraceElement.cs

    • Replaced JsonProperty with JsonPropertyName attributes.
    +5/-5     
    FirefoxProfile.cs
    Migrate FirefoxProfile to System.Text.Json                             

    dotnet/src/webdriver/Firefox/FirefoxProfile.cs

  • Replaced Newtonsoft.Json with System.Text.Json for JSON operations.
  • Updated JSON parsing logic.
  • +10/-2   
    DevToolsSessionEventReceivedEventArgs.cs
    Update DevToolsSessionEventReceivedEventArgs to use System.Text.Json

    dotnet/src/webdriver/DevTools/DevToolsSessionEventReceivedEventArgs.cs

    • Replaced JToken with JsonNode for event data.
    +3/-3     
    JavaScriptEngine.cs
    Migrate JavaScriptEngine to System.Text.Json                         

    dotnet/src/webdriver/JavaScriptEngine.cs

  • Replaced Newtonsoft.Json with System.Text.Json for JSON operations.
  • +2/-2     
    FirefoxExtension.cs
    Update FirefoxExtension to use System.Text.Json                   

    dotnet/src/webdriver/Firefox/FirefoxExtension.cs

    • Replaced JObject with JsonNode for JSON parsing.
    +2/-2     
    DriverOptions.cs
    Migrate DriverOptions to System.Text.Json                               

    dotnet/src/webdriver/DriverOptions.cs

  • Replaced Newtonsoft.Json with System.Text.Json for JSON serialization.

  • +2/-2     
    RemoteSessionSettings.cs
    Migrate RemoteSessionSettings to System.Text.Json               

    dotnet/src/webdriver/Remote/RemoteSessionSettings.cs

  • Replaced Newtonsoft.Json with System.Text.Json for JSON serialization.

  • +2/-2     
    ReturnedCapabilities.cs
    Migrate ReturnedCapabilities to System.Text.Json                 

    dotnet/src/webdriver/Internal/ReturnedCapabilities.cs

  • Replaced Newtonsoft.Json with System.Text.Json for JSON serialization.

  • +2/-2     
    command.hbs
    Update command template for System.Text.Json                         

    third_party/dotnet/devtools/src/generator/Templates/command.hbs

  • Replaced JsonProperty with JsonPropertyName and JsonIgnore attributes.

  • +5/-3     
    event.hbs
    Update event template for System.Text.Json                             

    third_party/dotnet/devtools/src/generator/Templates/event.hbs

  • Replaced JsonProperty with JsonPropertyName and JsonIgnore attributes.

  • +5/-4     
    type-object.hbs
    Update type-object template for System.Text.Json                 

    third_party/dotnet/devtools/src/generator/Templates/type-object.hbs

  • Replaced JsonProperty with JsonPropertyName and JsonIgnore attributes.

  • +3/-2     
    domain.hbs
    Update domain template for System.Text.Json                           

    third_party/dotnet/devtools/src/generator/Templates/domain.hbs

    • Replaced ToObject with Deserialize for event data.
    +2/-1     
    type-enum.hbs
    Update type-enum template for System.Text.Json                     

    third_party/dotnet/devtools/src/generator/Templates/type-enum.hbs

  • Replaced StringEnumConverter with custom JsonEnumMemberConverter.
  • +2/-3     
    type-hash.hbs
    Update type-hash template for System.Text.Json                     

    third_party/dotnet/devtools/src/generator/Templates/type-hash.hbs

  • Replaced JsonProperty with JsonPropertyName and JsonIgnore attributes.

  • +2/-1     
    Miscellaneous
    1 files
    ProxyTest.cs
    Remove unused Newtonsoft.Json imports in ProxyTest             

    dotnet/test/common/ProxyTest.cs

    • Commented out unused Newtonsoft.Json imports.
    +2/-127 
    Tests
    9 files
    TakesScreenshotTest.cs
    Update TakesScreenshotTest for .NET 8.0 compatibility       

    dotnet/test/common/TakesScreenshotTest.cs

    • Updated conditional compilation symbols to use NET8_0.
    +8/-8     
    WebDriver.IE.Tests.csproj
    Update IE tests to net8.0                                                               

    dotnet/test/ie/WebDriver.IE.Tests.csproj

    • Updated target framework to net8.0.
    +1/-1     
    WebDriver.Common.Tests.csproj
    Update common tests to net8.0                                                       

    dotnet/test/common/WebDriver.Common.Tests.csproj

    • Updated target framework to net8.0.
    +1/-1     
    WebDriver.Edge.Tests.csproj
    Update Edge tests to net8.0                                                           

    dotnet/test/edge/WebDriver.Edge.Tests.csproj

    • Updated target framework to net8.0.
    +1/-1     
    WebDriver.Firefox.Tests.csproj
    Update Firefox tests to net8.0                                                     

    dotnet/test/firefox/WebDriver.Firefox.Tests.csproj

    • Updated target framework to net8.0.
    +1/-1     
    WebDriver.Safari.Tests.csproj
    Update Safari tests to net8.0                                                       

    dotnet/test/safari/WebDriver.Safari.Tests.csproj

    • Updated target framework to net8.0.
    +1/-1     
    WebDriver.Chrome.Tests.csproj
    Update Chrome tests to net8.0                                                       

    dotnet/test/chrome/WebDriver.Chrome.Tests.csproj

    • Updated target framework to net8.0.
    +1/-1     
    WebDriver.Remote.Tests.csproj
    Update Remote tests to net8.0                                                       

    dotnet/test/remote/WebDriver.Remote.Tests.csproj

    • Updated target framework to net8.0.
    +1/-1     
    WebDriver.Support.Tests.csproj
    Update Support tests to net8.0                                                     

    dotnet/test/support/WebDriver.Support.Tests.csproj

    • Updated target framework to net8.0.
    +1/-1     
    Dependencies
    3 files
    paket.nuget.bzl
    Update System.Text.Json package version                                   

    dotnet/paket.nuget.bzl

    • Updated System.Text.Json package version to 8.0.4.
    +2/-2     
    WebDriver.csproj
    Update WebDriver.csproj for System.Text.Json                         

    dotnet/src/webdriver/WebDriver.csproj

  • Replaced Newtonsoft.Json with System.Text.Json for netstandard2.0.
  • +2/-2     
    paket.dependencies
    Update System.Text.Json package version                                   

    dotnet/paket.dependencies

    • Updated System.Text.Json package version to 8.0.4.
    +1/-1     
    Configuration changes
    6 files
    BUILD.bazel
    Add net8.0 targets and update dependencies                             

    dotnet/src/webdriver/BUILD.bazel

  • Added new targets for net8.0 framework.
  • Updated dependencies to use System.Text.Json.
  • +74/-12 
    WebDriver.StrongNamed.nuspec
    Update nuspec for System.Text.Json and net8.0                       

    dotnet/src/webdriver/WebDriver.StrongNamed.nuspec

  • Replaced Newtonsoft.Json dependency with System.Text.Json.
  • Added net8.0 target framework.
  • +8/-1     
    WebDriver.nuspec
    Update nuspec for System.Text.Json and net8.0                       

    dotnet/src/webdriver/WebDriver.nuspec

  • Replaced Newtonsoft.Json dependency with System.Text.Json.
  • Added net8.0 target framework.
  • +8/-1     
    BUILD.bazel
    Update common test build for net8.0                                           

    dotnet/test/common/BUILD.bazel

  • Updated target framework to net8.0.
  • Updated dependencies to use webdriver-net8.0.
  • +4/-4     
    BUILD.bazel
    Update support build for netstandard2.0                                   

    dotnet/src/support/BUILD.bazel

    • Updated dependencies to use webdriver-netstandard2.0.
    +3/-4     
    MODULE.bazel
    Update .NET toolchain version to 8.0.203                                 

    MODULE.bazel

    • Updated .NET toolchain version to 8.0.203.
    +1/-1     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link
    Contributor

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Serialization Changes
    The migration from Newtonsoft.Json to System.Text.Json involves changes in serialization and deserialization methods which could potentially lead to issues if not handled correctly. Ensure that all data types and formats are correctly managed with the new library, especially in complex nested objects or where specific formatting is required.

    Error Handling
    The error handling logic has been modified to use System.Text.Json. It's crucial to ensure that the error handling still correctly parses and responds to error conditions as expected.

    Attribute Changes
    The attributes for serialization have been changed from Newtonsoft.Json attributes to System.Text.Json attributes. Verify that the new attributes (JsonPropertyName, JsonIgnore) are correctly applied and behave as expected in all scenarios, including default handling and null value handling.

    Serialization Attributes
    The transition to System.Text.Json involves changes to serialization attributes. Ensure that these changes do not affect the serialization and deserialization of Cookie objects, particularly with respect to handling default values and null handling.

    Copy link
    Contributor

    qodo-merge-pro bot commented Jul 22, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Add JsonSerializerOptions to handle potential deserialization issues

    When using JsonSerializer.Deserialize to deserialize JSON data, it's a good practice
    to specify JsonSerializerOptions to handle potential issues like case sensitivity
    and missing members. This is particularly important when migrating from
    Newtonsoft.Json to System.Text.Json as they handle defaults differently.

    dotnet/src/webdriver/DevTools/DevToolsSession.cs [404]

    -var versionInfo = JsonSerializer.Deserialize<DevToolsVersionInfo>(rawVersionInfo);
    +var options = new JsonSerializerOptions { PropertyNameCaseInsensitive = true };
    +var versionInfo = JsonSerializer.Deserialize<DevToolsVersionInfo>(rawVersionInfo, options);
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: The suggestion addresses a potential issue with deserialization that can arise due to differences between Newtonsoft.Json and System.Text.Json. Adding JsonSerializerOptions improves robustness and compatibility, which is crucial when migrating libraries.

    9
    Possible issue
    Add error handling for JSON deserialization to improve robustness

    Consider adding error handling for the deserialization process to prevent runtime
    exceptions if the JSON format is incorrect.

    dotnet/src/webdriver/Response.cs [151]

    -Dictionary<string, object> deserializedResponse = JsonSerializer.Deserialize<Dictionary<string, object>>(value, jsonSerializerOptions);
    +Dictionary<string, object> deserializedResponse;
    +try
    +{
    +    deserializedResponse = JsonSerializer.Deserialize<Dictionary<string, object>>(value, jsonSerializerOptions);
    +}
    +catch (JsonException e)
    +{
    +    throw new InvalidOperationException("Failed to deserialize response.", e);
    +}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: Adding error handling for JSON deserialization is crucial to prevent runtime exceptions and improve the robustness of the code. This change addresses a significant potential issue.

    9
    Add missing dependency versions for newer .NET frameworks to ensure compatibility

    It's recommended to specify the dependency versions for
    System.Threading.Tasks.Extensions in the net5.0, net6.0, net7.0, and net8.0
    frameworks to ensure compatibility and prevent potential runtime issues due to
    missing dependencies.

    dotnet/paket.nuget.bzl [16]

    -"dependencies": {"net11": [], "net20": [], "net30": [], "net35": [], "net40": [], "net403": [], "net45": [], "net451": [], "net452": [], "net46": [], "net461": ["System.Threading.Tasks.Extensions"], "net462": ["System.Threading.Tasks.Extensions"], "net47": ["System.Threading.Tasks.Extensions"], "net471": ["System.Threading.Tasks.Extensions"], "net472": ["System.Threading.Tasks.Extensions"], "net48": ["System.Threading.Tasks.Extensions"], "net5.0": [], "net6.0": [], "net7.0": [], "net8.0": [], "netcoreapp1.0": [], "netcoreapp1.1": [], "netcoreapp2.0": ["System.Threading.Tasks.Extensions"], "netcoreapp2.1": ["System.Threading.Tasks.Extensions"], "netcoreapp2.2": ["System.Threading.Tasks.Extensions"], "netcoreapp3.0": [], "netcoreapp3.1": [], "netstandard": [], "netstandard1.0": [], "netstandard1.1": [], "netstandard1.2": [], "netstandard1.3": [], "netstandard1.4": [], "netstandard1.5": [], "netstandard1.6": [], "netstandard2.0": ["System.Threading.Tasks.Extensions"], "netstandard2.1": []}
    +"dependencies": {"net11": [], "net20": [], "net30": [], "net35": [], "net40": [], "net403": [], "net45": [], "net451": [], "net452": [], "net46": [], "net461": ["System.Threading.Tasks.Extensions"], "net462": ["System.Threading.Tasks.Extensions"], "net47": ["System.Threading.Tasks.Extensions"], "net471": ["System.Threading.Tasks.Extensions"], "net472": ["System.Threading.Tasks.Extensions"], "net48": ["System.Threading.Tasks.Extensions"], "net5.0": ["System.Threading.Tasks.Extensions"], "net6.0": ["System.Threading.Tasks.Extensions"], "net7.0": ["System.Threading.Tasks.Extensions"], "net8.0": ["System.Threading.Tasks.Extensions"], "netcoreapp1.0": [], "netcoreapp1.1": [], "netcoreapp2.0": ["System.Threading.Tasks.Extensions"], "netcoreapp2.1": ["System.Threading.Tasks.Extensions"], "netcoreapp2.2": ["System.Threading.Tasks.Extensions"], "netcoreapp3.0": [], "netcoreapp3.1": [], "netstandard": [], "netstandard1.0": [], "netstandard1.1": [], "netstandard1.2": [], "netstandard1.3": [], "netstandard1.4": [], "netstandard1.5": [], "netstandard1.6": [], "netstandard2.0": ["System.Threading.Tasks.Extensions"], "netstandard2.1": []}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 9

    Why: The suggestion correctly identifies a potential issue with missing dependencies for newer .NET frameworks, which could lead to runtime issues. Adding these dependencies improves compatibility and robustness of the code.

    9
    Add error handling for potential serialization failures

    Consider handling the case where JsonSerializer.Serialize(writer, value, options)
    throws an exception due to serialization issues. This can be done by wrapping the
    call in a try-catch block and handling the exception appropriately, possibly by
    logging the error or rethrowing a more informative exception.

    dotnet/src/webdriver/Internal/ResponseValueJsonConverter.cs [38]

    -JsonSerializer.Serialize(writer, value, options);
    +try
    +{
    +    JsonSerializer.Serialize(writer, value, options);
    +}
    +catch (Exception ex)
    +{
    +    throw new JsonException("Failed to serialize value.", ex);
    +}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding error handling for serialization failures is a good practice to ensure robustness and provide informative error messages. This suggestion addresses a potential issue that could cause runtime errors.

    8
    Add key existence checks to prevent runtime exceptions

    Validate the presence of keys in the deserialized dictionary to prevent potential
    KeyNotFoundException.

    dotnet/src/webdriver/Firefox/FirefoxProfile.cs [312-313]

    -Dictionary<string, object> immutableDefaultPreferences = deserializedPreferences["frozen"] as Dictionary<string, object>;
    -Dictionary<string, object> editableDefaultPreferences = deserializedPreferences["mutable"] as Dictionary<string, object>;
    +deserializedPreferences.TryGetValue("frozen", out Dictionary<string, object> immutableDefaultPreferences);
    +deserializedPreferences.TryGetValue("mutable", out Dictionary<string, object> editableDefaultPreferences);
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Validating the presence of keys in the deserialized dictionary prevents potential KeyNotFoundException, enhancing the code's reliability and robustness.

    8
    Enhancement
    Improve error handling in JSON enum conversion to avoid silent failures

    Add error handling for cases where the JSON value does not match any enum values to
    prevent returning default values without notice.

    dotnet/src/webdriver/DevTools/Json/JsonEnumMemberConverter.cs [50]

    -return default;
    +throw new JsonException($"Unknown value: {stringValue} for enum type: {typeof(TEnum)}");
     
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    Why: Adding error handling for unknown enum values prevents silent failures and makes debugging easier, improving the overall code quality.

    8
    Use JsonSerializer.Deserialize for JSON parsing instead of manual handling

    Replace the direct instantiation of Dictionary<string, object> and List with calls
    to JsonSerializer.Deserialize to leverage System.Text.Json's capabilities to handle
    different JSON structures and improve performance and maintainability.

    dotnet/src/webdriver/Internal/ResponseValueJsonConverter.cs [48-60]

    -Dictionary<string, object> dictionaryValue = new Dictionary<string, object>();
    -List<object> arrayValue = new List<object>();
    +var dictionaryValue = JsonSerializer.Deserialize<Dictionary<string, object>>(ref reader, options);
    +var arrayValue = JsonSerializer.Deserialize<List<object>>(ref reader, options);
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Using JsonSerializer.Deserialize can improve performance and maintainability by leveraging built-in capabilities for JSON parsing. However, it may require additional validation to ensure it handles all cases correctly.

    7
    Enhance the exception message with more detailed context

    Improve the exception message in the ProcessToken method by including more context
    about the error, such as the current position in the JSON document, to help with
    debugging.

    dotnet/src/webdriver/Internal/ResponseValueJsonConverter.cs [101]

    -throw new JsonException($"Unrecognized '{reader.TokenType}' token type while parsing command response.");
    +throw new JsonException($"Unrecognized token type '{reader.TokenType}' at position {reader.TokenStartIndex} while parsing command response.");
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Improving the exception message with more context can significantly aid in debugging by providing more information about where the error occurred. This is a useful enhancement.

    7
    Possible bug
    Ensure correct null handling in JSON serialization for SerializableLatency

    Ensure that the SerializableLatency property correctly handles null values when
    serializing, as the JsonIgnore attribute with condition might not be sufficient.

    dotnet/src/webdriver/Chromium/ChromiumNetworkConditions.cs [91-92]

     [JsonPropertyName("latency")]
    -[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
    -internal long? SerializableLatency
    +internal long? SerializableLatency => this.latency.HasValue ? this.latency : null;
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: The suggestion improves the handling of null values during serialization, which is important for data integrity. However, the existing code already uses JsonIgnoreCondition.WhenWritingNull, which is generally sufficient.

    7
    Maintainability
    Refactor ProcessToken into smaller methods for better readability and maintainability

    Refactor the ProcessToken method to separate concerns and improve readability by
    breaking down the method into smaller, more focused methods, each handling a
    specific token type.

    dotnet/src/webdriver/Internal/ResponseValueJsonConverter.cs [41-104]

     private object ProcessToken(ref Utf8JsonReader reader, JsonSerializerOptions options)
     {
    -    // method implementation
    +    switch (reader.TokenType)
    +    {
    +        case JsonTokenType.StartObject:
    +            return ProcessJsonObject(ref reader, options);
    +        case JsonTokenType.StartArray:
    +            return ProcessJsonArray(ref reader, options);
    +        // other cases
    +    }
     }
     
    +private object ProcessJsonObject(ref Utf8JsonReader reader, JsonSerializerOptions options)
    +{
    +    // handle JsonTokenType.StartObject
    +}
    +
    +private object ProcessJsonArray(ref Utf8JsonReader reader, JsonSerializerOptions options)
    +{
    +    // handle JsonTokenType.StartArray
    +}
    +
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Refactoring the ProcessToken method into smaller methods can improve readability and maintainability. This is a good practice but not critical for functionality.

    6

    @nvborisenko
    Copy link
    Member Author

    @SeleniumHQ/selenium-committers this is ready for review.

    @nvborisenko nvborisenko merged commit 77010cd into SeleniumHQ:trunk Jul 25, 2024
    3 of 4 checks passed
    sandeepsuryaprasad pushed a commit to sandeepsuryaprasad/selenium that referenced this pull request Oct 29, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    [🚀 Feature]: Replace Newtonsoft.Json package with System.Text.Json
    2 participants